Ensure that create script installs same version of starter files and kit #1621
Conversation
Install packages before copying files.
lfdebrux
force-pushed
the
ldeb-fix-issue-1616
branch
2 times, most recently
from
99874c8 to
712c731
Compare
Splits the process of creating a new prototype into two with copying the starter files done after the version of the kit to be used has been installed. This makes sure the user doesn't end up with starter files that require features not available to the installed kit version. This is similar to what the create-react-app tool does [[1]]. [1]: https://github.com/facebook/create-react-app/blob/main/packages/create-react-app/createReactApp.js
lfdebrux
force-pushed
the
ldeb-fix-issue-1616
branch
from
712c731 to
5868df4
Compare
bin/cli
Outdated
| // as possible into stage two; stage one should ideally be able to install | ||
| // any future version of the kit. | ||
|
|
||
| const stage = additionalOptions[0] === '--' ? 'two' : 'one' |
There was a problem hiding this comment.
Rather than having the stage one and two both called create can we call stage one create and stage two generate-starter-files (or something similar)?
Also, I realised we had a problem with the cli args and I've created a util for reading them. Feel free to bring these changes into your branch if it helps (if you do I'll close my PR) https://github.com/alphagov/govuk-prototype-kit/pull/1622/files
There was a problem hiding this comment.
Rather than having the stage one and two both called
createcan we call stage one create and stage twogenerate-starter-files(or something similar)?
Yeah we can do that. Do we want that stage two command to be documented and usable independently?
There was a problem hiding this comment.
Commit f6f063c does this, if you're happy with the result I'll squash it into the parent.
| // `init` is stage two of the install process (see above), it should be | ||
| // called by `create` with the correct arguments. | ||
|
|
||
| if (additionalOptions[0] !== '--') { |
There was a problem hiding this comment.
What's the reason for the double-hypen? Is it just to make sure people don't use it by mistake?
There was a problem hiding this comment.
I'm wondering whether we should be more specific with it - something like --internal-use-only ... selfishly for the parser I've just created I'd love it if it was a key and value pair e.g. ---internal-use=true.
If you think it's best as just -- we can support that separately to the parser.
There was a problem hiding this comment.
I'm wondering whether we should be more specific with it
I'm not sure there is any need to be more specific, using -- to indicate internal arguments is a pattern I've encountered several times before in the past.
If you think it's best as just -- we can support that separately to the parser.
Yeah, I don't think it needs to go into the parser, this code can read the arguments straightforwardly from process.argv and assume that the caller has supplied them correctly.
There was a problem hiding this comment.
Cool, I'm happy with that.
Splits the process of creating a new prototype into two with copying the starter files done after the version of the kit to be used has been installed. This makes sure the user doesn't end up with starter files that require features not available to the installed kit version. This is similar to what the create-react-app tool does [1].
Fixes #1616.